-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ISSUE-16 - Refactor cli module #111
Conversation
Split cli module into component parts. Create: * find module * format - table module * format - tree module * rs_file module * scan module * traversal module Signed-off-by: Josh McConnell <jmcconnell26@qub.ac.uk>
At a first glance this looks like a great improvement! Thanks! |
cargo-geiger/src/cli.rs
Outdated
use std::sync::Arc; | ||
use std::sync::Mutex; | ||
use walkdir::DirEntry; | ||
use walkdir::WalkDir; | ||
|
||
// ---------- BEGIN: Public items ---------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove these comments now when modules are much smaller. I haven't seen similar comment sections in other Rust projects so unless you insist to keep them let's remove them all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a good shout, I had kept them in to keep a consistent style, but am happy to take them out.
geiger/src/lib.rs
Outdated
pub enum DetectionStatus { | ||
NoneDetectedForbidsUnsafe, | ||
NoneDetectedAllowsUnsafe, | ||
UnsafeDetected, | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this type should probably be renamed to something like CrateDetectionStatus
and moved back to the cargo-geiger
crate since the geiger
library doesn't know about crates, only files.
Thanks for the review! |
I have just made the changes, would they be better in a separate commit, or would you rather I amend the first? |
A new commit can be easier to review sometimes. |
Make the following changes: * Remove all "BEGIN: Public items" comments * Move DetectionStatus enum back to cargo-geiger and rename to CrateDetectionStatus Signed-off-by: Josh McConnell <jmcconnell26@qub.ac.uk>
Good work, thanks again! |
Split cli module into component parts. Create:
Signed-off-by: Josh McConnell jmcconnell26@qub.ac.uk